[WIP] feat: support SMB mount with managed identity #2652

andyzhangx · 2025-07-05T02:46:20Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
feat: support SMB mount with managed identity

Which issue(s) this PR fixes:

Fixes #

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
tested upgrade from previous version

Special notes for your reviewer:

# k exec -it -n kube-system csi-azurefile-node-ls278 -c azurefile -- sh
# azfilesrefresh
Traceback (most recent call last):
  File "/usr/bin/azfilesrefresh", line 3, in <module>
    from azfilesauth import azfiles_set_oauth, get_oauth_token
ModuleNotFoundError: No module named 'azfilesauth'
# azfilesauthmanager

Usage:
        azfilesauthmanager list
        azfilesauthmanager set <file_endpoint_uri> <oauth_token>
        azfilesauthmanager set <file_endpoint_uri> --imds-client-id <client_id>
        azfilesauthmanager clear <file_endpoint_uri>

# ls -lt /usr/bin/azfile*
-rwxr-xr-x 1 root root 6862 May  3 12:00 /usr/bin/azfilesauthmanager
-rwxr-xr-x 1 root root 4649 May  3 12:00 /usr/bin/azfilesrefresh

Release note:

feat: support SMB mount with managed identity

k8s-ci-robot · 2025-07-05T02:46:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andyzhangx

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [andyzhangx]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

andyzhangx · 2025-07-16T09:53:42Z

/retest

chore: add kubelet mount path chore: mkdir kubeberos dir fix fix install sudo curl revert fix mountOptions support clientID support clientID in sc fix sh script golint error fix fix config file creation on host add SETUP_MI_AUTH env var fix azfilesauth_1.0 package add azfilesrefresh sidecar feat: mount with managed identity auth fix

fix chart config

Copilot

Pull Request Overview

This PR adds support for SMB mount with managed identity authentication in the Azure File CSI driver. The changes enable using Azure managed identity for authentication instead of traditional storage account keys.

Key changes:

Adds a new mountwithmanagedidentity volume context parameter to enable managed identity authentication
Installs and integrates azfilesauthmanager tool for credential caching in the container images
Updates mount logic to use Kerberos authentication (sec=krb5) with managed identity when enabled

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
pkg/azurefileplugin/Dockerfile	Adds azfilesauth package installation and required dependencies for managed identity support
pkg/azurefile/utils.go	Implements credential cache setup function using azfilesauthmanager
pkg/azurefile/nodeserver.go	Updates mount logic to support managed identity authentication with Kerberos
pkg/azurefile/controllerserver.go	Adds mountwithmanagedidentity field validation in CreateVolume
pkg/azurefile/azurefile.go	Adds mountwithmanagedidentity constant and account info handling logic
pkg/azurefile-proxy/init.sh	Sets up Kerberos configuration files for managed identity authentication
deploy/csi-azurefile-node.yaml	Adds azfilesrefresh sidecar container and required volume mounts
charts/latest/azurefile-csi-driver/values.yaml	Adds configuration option to enable managed identity authentication
charts/latest/azurefile-csi-driver/templates/csi-azurefile-node.yaml	Updates Helm template with conditional azfilesrefresh container

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-18T11:21:05Z

pkg/azurefileplugin/Dockerfile

@@ -22,6 +22,7 @@ ARG ARCH

 RUN apt update \
    && apt install -y curl \
+    && curl -Lso /tmp/azfilesauth_amd64.deb https://raw.githubusercontent.com/andyzhangx/demo/refs/heads/master/aks/azfilesauth_1.0-4_${ARCH}.deb \


Downloading packages from a personal GitHub repository (andyzhangx/demo) poses a security risk. Consider hosting the package in an official repository or verified location.

Suggested change

&& curl -Lso /tmp/azfilesauth_amd64.deb https://raw.githubusercontent.com/andyzhangx/demo/refs/heads/master/aks/azfilesauth_1.0-4_${ARCH}.deb \

&& curl -Lso /tmp/azfilesauth_amd64.deb https://github.com/andyzhangx/demo/releases/download/v1.0-4/azfilesauth_1.0-4_${ARCH}.deb \

Copilot · 2025-08-18T11:21:06Z

pkg/azurefileplugin/Dockerfile

 RUN chmod +x /azurefile-proxy/*.sh && \
  chmod +x /azurefile-proxy/azurefile-proxy.service && \
  chmod +x /azurefile-proxy/azurefile-proxy

+RUN if [ "$ARCH" = "amd64" ] ; then \
+  clean-install libcurl4-gnutls-dev \
+  && apt update && apt install -y /azurefile-proxy/azfilesauth_amd64.deb; fi


The package installation is hardcoded to only work for amd64 architecture, but the download URL uses ${ARCH} variable. This creates an inconsistency where other architectures will download the package but not install it.

Suggested change

&& apt update && apt install -y /azurefile-proxy/azfilesauth_amd64.deb; fi

COPY --from=builder --chown=root:root /tmp/azfilesauth_${ARCH}.deb /azurefile-proxy/azfilesauth_${ARCH}.deb

RUN chmod +x /azurefile-proxy/*.sh && \

chmod +x /azurefile-proxy/azurefile-proxy.service && \

chmod +x /azurefile-proxy/azurefile-proxy

RUN clean-install libcurl4-gnutls-dev \

&& apt update && apt install -y /azurefile-proxy/azfilesauth_${ARCH}.deb

Copilot · 2025-08-18T11:21:06Z

pkg/azurefile/nodeserver.go

+				clientID = d.cloud.Config.AzureAuthConfig.UserAssignedIdentityID
+			}
+			sensitiveMountOptions = []string{"sec=krb5,cruid=0,upcall_target=mount", fmt.Sprintf("username=%s", clientID)}
+			klog.V(2).Infof("using managed identity %s for volume %s with mount options: %v", clientID, volumeID, sensitiveMountOptions)
 		} else {


[nitpick] The nested if-else structure creates complex control flow. Consider restructuring to handle the different authentication methods (NFS, managed identity, traditional) in separate conditional blocks for better readability.

Copilot · 2025-08-18T11:21:06Z

pkg/azurefile/nodeserver.go

@@ -313,7 +311,6 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
 			fileShareNameReplaceMap[pvNameMetadata] = v
 		case mountPermissionsField:
 			if v != "" {
-				var err error
 				var perm uint64
 				if perm, err = strconv.ParseUint(v, 8, 32); err != nil {


The err variable is used without being declared. This will cause a compilation error since err was removed from the variable declaration on the previous lines.

Copilot · 2025-08-18T11:21:07Z

pkg/azurefile/utils.go

+	cmd.Env = append(os.Environ(), cmd.Env...)
+	klog.V(2).Infof("Executing command: %q", cmd.String())
+	return cmd.CombinedOutput()
+}


The clientID parameter should be validated before being passed to exec.Command to prevent command injection attacks, especially since it could come from user-provided volume context.

k8s-ci-robot · 2025-08-20T18:02:59Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 5, 2025

k8s-ci-robot requested review from cvvz and ZeroMagic July 5, 2025 02:46

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 5, 2025

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 13, 2025

andyzhangx force-pushed the mi branch from 2b18345 to 3d16cf0 Compare July 16, 2025 08:46

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 20, 2025

andyzhangx force-pushed the mi branch from bd05f47 to 28c0679 Compare July 21, 2025 02:40

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2025

andyzhangx force-pushed the mi branch 2 times, most recently from 08d98cd to 5564b5c Compare July 21, 2025 14:03

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2025

andyzhangx force-pushed the mi branch from 5564b5c to f8f4279 Compare July 22, 2025 08:10

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2025

andyzhangx mentioned this pull request Jul 24, 2025

Connection to storage account with storage account keys disabled doesn't work with workload identity (kerberos auth support) #1737

Open

andyzhangx added 3 commits August 8, 2025 16:25

add /var/log/ mountPath

3108f45

fix chart config

rename

8829136

andyzhangx force-pushed the mi branch from e670d2f to 8829136 Compare August 8, 2025 16:26

fix image build failure

3b781e9

andyzhangx requested review from Copilot and removed request for ZeroMagic August 18, 2025 11:18

Copilot AI reviewed Aug 18, 2025

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[WIP] feat: support SMB mount with managed identity #2652

[WIP] feat: support SMB mount with managed identity #2652

andyzhangx commented Jul 5, 2025 •

edited

Loading

Uh oh!

k8s-ci-robot commented Jul 5, 2025

Uh oh!

andyzhangx commented Jul 16, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

Copilot AI Aug 18, 2025

Uh oh!

k8s-ci-robot commented Aug 20, 2025

Uh oh!

Uh oh!

	&& curl -Lso /tmp/azfilesauth_amd64.deb https://raw.githubusercontent.com/andyzhangx/demo/refs/heads/master/aks/azfilesauth_1.0-4_${ARCH}.deb \
	&& curl -Lso /tmp/azfilesauth_amd64.deb https://github.com/andyzhangx/demo/releases/download/v1.0-4/azfilesauth_1.0-4_${ARCH}.deb \

[WIP] feat: support SMB mount with managed identity #2652

Are you sure you want to change the base?

[WIP] feat: support SMB mount with managed identity #2652

Conversation

andyzhangx commented Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

k8s-ci-robot commented Jul 5, 2025

Uh oh!

andyzhangx commented Jul 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 18, 2025

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Aug 20, 2025

Uh oh!

Uh oh!

andyzhangx commented Jul 5, 2025 •

edited

Loading